fix(ci): isolate LLVM profile outputs#20
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ad37d4a5e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| run: | | ||
| cd build/tests | ||
| ctest --output-on-failure -j$(sysctl -n hw.ncpu) | ||
| LLVM_PROFILE_FILE="$PWD/%p-%m.profraw" ctest --output-on-failure -j$(sysctl -n hw.ncpu) |
There was a problem hiding this comment.
This only sets LLVM_PROFILE_FILE for the ctest invocation, but the preceding Build Tests step also runs many instrumented test executables because tests/CMakeLists.txt uses gtest_discover_tests(...) throughout, and CMake's default DISCOVERY_MODE is POST_BUILD (it runs each binary with --gtest_list_tests at build time). Under cmake --build ... -j, those discovery processes can still concurrently write build/tests/default.profraw, and the later llvm-profdata merge build/tests/*.profraw will include that same potentially corrupted file, so the CI coverage failure this patch targets can still occur before this environment assignment ever applies.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Codex Review: Didn't find any major issues. Keep them coming!
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Thanks, looks valid |
fix(ci): isolate LLVM profile outputs
DICE and main had diverged into two parallel lines. This merge records DICE (40a796c) as a second parent so main's prior history — including gly11's merged PRs #18/#19/#20 and the sbp2/ci/foundation fixes — is preserved as ancestry, while the resulting tree is taken wholesale from DICE. main-only code is superseded by DICE's implementation but remains recoverable from history (refs/backup/main-pre-dice-merge = 757456d). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2 participants
Summary
This PR makes the C++ coverage step deterministic when tests run in parallel. The workflow currently lets every instrumented test process write to the default
default.profraw, which can produce mismatched or corrupted profile data underctest -j.Setting
LLVM_PROFILE_FILEto include process/module placeholders gives each test process its own profile output beforellvm-profdata merge.Context
This showed up on #19 after all 480 tests passed, when coverage processing failed with:
A previous successful run on the same source tree already reported mismatched profile data, so this is a CI coverage stability fix rather than a product-code change.
Verification